Unify BPF verifiers #177
Unify BPF verifiers #177
Conversation
jackcmay
force-pushed
the
consolidate-bpf-verifier
branch
2 times, most recently
from
5e3efaf to
5b97b3f
Compare
| @@ -258,7 +253,7 @@ pub fn check(prog: &[u8]) -> Result<(), UserError> { | |||
| ebpf::ADD64_REG => {}, | |||
| ebpf::SUB64_IMM => {}, | |||
| ebpf::SUB64_REG => {}, | |||
| ebpf::MUL64_IMM => {}, | |||
| ebpf::MUL64_IMM => { check_imm_nonzero(&insn, insn_ptr)?; }, | |||
There was a problem hiding this comment.
Yes, stale in rbpf
There was a problem hiding this comment.
Actually, this was removed here but not in the mono repo I think.
And I would carry the removal forward as MUL32_IMM also has no such limit.
There was a problem hiding this comment.
Great, I'll take a look if we can remove it from the monorepo
There was a problem hiding this comment.
We should remove from the monorepo in s separate PR, probably with featurization, tracking here: solana-labs/solana#17520
jackcmay
force-pushed
the
consolidate-bpf-verifier
branch
from
5b97b3f to
014ce37
Compare
jackcmay
force-pushed
the
consolidate-bpf-verifier
branch
from
014ce37 to
38927bd
Compare
There was a problem hiding this comment.
What about the changes related to clippy::upper_case_acronyms?
And this added space here:
Line 62 in 38927bd
Otherwise I think it is ready to go.
tests/ubpf_execution.rs
Outdated
| @@ -61,7 +59,7 @@ macro_rules! test_interpreter_and_jit { | |||
| $(test_interpreter_and_jit!(bind, vm, $location => $syscall_function; $syscall_context_object);)* | |||
| let result = vm.execute_program_jit(&mut TestInstructionMeter { remaining: $expected_instruction_count }); | |||
| let tracer_jit = vm.get_tracer(); | |||
| if !check_closure(&vm, result) || !solana_rbpf::vm::Tracer::compare(&_tracer_interpreter, tracer_jit) { | |||
| if !check_closure(&vm, result) || !solana_rbpf::vm::Tracer::compare(&_tracer_interpreter, tracer_jit) { | |||
There was a problem hiding this comment.
cargo fmt sucks in macros
I like the upper case for things like instruction names "LDDW" rather than "Lddw" and think this is a good place to make that exception. But, I could go either way if you feel stronger about it |
|
I liked them as well, but in other places we also go with the clippy flow. |
jackcmay
force-pushed
the
consolidate-bpf-verifier
branch
from
38927bd to
79f5375
Compare
Apparently clippy no longer cares... |
There are currently two bpf verifiers, one in the rbpf crate and one in the bpf_loader crate in the Solana repo. Only the one in the bpf_loader crate is used by the cluster but having two is confusing.
This PR pulls the verifier from the bpf_loader crate and makes the necessary changes in prep for the bpf_loader crate to switch over to the updated one in the rbpf create.